Skip to content

[VectorCombine] Shrink loads used in shufflevector rebroadcasts. #153138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Aug 12, 2025

Conversation

PeddleSpam
Copy link
Contributor

Reopen #128938.

Attempt to shrink the size of vector loads where only some of the incoming lanes are used for rebroadcasts in shufflevector instructions.

Copy link

github-actions bot commented Aug 12, 2025

✅ With the latest revision this PR passed the undef deprecator.

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - cheers

@PeddleSpam PeddleSpam merged commit 9115bef into llvm:main Aug 12, 2025
9 checks passed
@mstorsjo
Copy link
Member

This triggers failed asserts when compiling for ARM:

short *ScaleUVRowDown2_C_src, *ScaleUVRowDown2_C_dst;
int ScaleUVRowDown2_C_x;
void ScaleUVRowDown2_C() {
  for (; ScaleUVRowDown2_C_x; ScaleUVRowDown2_C_x += 2) {
    ScaleUVRowDown2_C_dst[0] = ScaleUVRowDown2_C_src[1];
    ScaleUVRowDown2_C_dst[1] = ScaleUVRowDown2_C_src[3];
    ScaleUVRowDown2_C_src += 2;
    ScaleUVRowDown2_C_dst += 2;
  }
}
$ clang -target armv7-linux-gnueabihf -c repro.c -O2
clang: ../include/llvm/ADT/ArrayRef.h:253: const T& llvm::ArrayRef<T>::operator[](size_t) const [with T = int; size_t = long unsigned int]: Assertion `Index < Length && "Invalid index!"' failed.

@mstorsjo
Copy link
Member

Also, note the commit https://github.com/llvm/llvm-project/commit/9115bef8ee1d9a47fef3221d9f3e0fd5c4ed947c.patch ended up with a hidden email. Please turn off Keep my email addresses private setting in your account. See LLVM Developer Policy and LLVM Discourse for more information.

@PeddleSpam
Copy link
Contributor Author

This triggers failed asserts when compiling for ARM:

short *ScaleUVRowDown2_C_src, *ScaleUVRowDown2_C_dst;
int ScaleUVRowDown2_C_x;
void ScaleUVRowDown2_C() {
  for (; ScaleUVRowDown2_C_x; ScaleUVRowDown2_C_x += 2) {
    ScaleUVRowDown2_C_dst[0] = ScaleUVRowDown2_C_src[1];
    ScaleUVRowDown2_C_dst[1] = ScaleUVRowDown2_C_src[3];
    ScaleUVRowDown2_C_src += 2;
    ScaleUVRowDown2_C_dst += 2;
  }
}
$ clang -target armv7-linux-gnueabihf -c repro.c -O2
clang: ../include/llvm/ADT/ArrayRef.h:253: const T& llvm::ArrayRef<T>::operator[](size_t) const [with T = int; size_t = long unsigned int]: Assertion `Index < Length && "Invalid index!"' failed.

This seems to be a bug in the ARM backend. The generated shufflevector is valid (see below), but the function isVTRNMask in ARMISelLowering.cpp assumes the mask size is equal to, or exactly double, the size of the input vector.

%17 = load <15 x i16>, ptr %16, align 2, !tbaa !9
%18 = getelementptr inbounds nuw i8, ptr %next.gep, i32 6
%19 = load <15 x i16>, ptr %18, align 2, !tbaa !9
%interleaved.vec = shufflevector <15 x i16> %17, <15 x i16> %19, <16 x i32> <i32 0, i32 15, i32 2, i32 17, i32 4, i32 19, i32 6, i32 21, i32 8, i32 23, i32 10, i32 25, i32 12, i32 27, i32 14, i32 29>

@mstorsjo
Copy link
Member

This seems to be a bug in the ARM backend. The generated shufflevector is valid (see below), but the function isVTRNMask in ARMISelLowering.cpp assumes the mask size is equal to, or exactly double, the size of the input vector.

%17 = load <15 x i16>, ptr %16, align 2, !tbaa !9
%18 = getelementptr inbounds nuw i8, ptr %next.gep, i32 6
%19 = load <15 x i16>, ptr %18, align 2, !tbaa !9
%interleaved.vec = shufflevector <15 x i16> %17, <15 x i16> %19, <16 x i32> <i32 0, i32 15, i32 2, i32 17, i32 4, i32 19, i32 6, i32 21, i32 8, i32 23, i32 10, i32 25, i32 12, i32 27, i32 14, i32 29>

CC @davemgreen @DavidSpickett

Even if this is an unrelated backend bug, I'd like to revert this change, that exposes the preexisting issue, in order to get back to green until we have the backend bug fixed.

@davemgreen
Copy link
Collaborator

I'll take a look.

PeddleSpam pushed a commit to PeddleSpam/llvm-project that referenced this pull request Aug 13, 2025
davemgreen added a commit to davemgreen/llvm-project that referenced this pull request Aug 13, 2025
Fixes the issue reported on llvm#153138, where odd-sized vectors would cause the
checks to iterate off the end of the mask.
davemgreen added a commit that referenced this pull request Aug 13, 2025
…53413)

Fixes the issue reported on #153138, where odd-sized vectors would cause
the checks to iterate off the end of the mask.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants